Automated Code Review

Reviewers: 3 | Confidence: 85%

✓ Correctness

This is a clean type-narrowing refactor that replaces the broader SupportsAgentRun protocol with the concrete Agent class throughout the handoff module. The runtime isinstance check in participants() was correctly updated, and a new test validates the stricter behavior. The only notable issue is an inconsistent docstring in with_autonomous_mode that now mentions both SupportsAgentRun and Agent despite the type having been narrowed to Agent only. No correctness bugs were found.

✓ Security Reliability

This diff narrows the accepted type from the SupportsAgentRun protocol to the concrete Agent class throughout the handoff system. The public entry point (HandoffBuilder.participants) retains a runtime isinstance check, but the runtime guard inside _prepare_agent_with_handoffs was removed. Since HandoffAgentExecutor is a public class whose init now accepts agent: Agent without any runtime validation, a caller bypassing HandoffBuilder could pass a non-Agent object and hit undefined behavior in _clone_chat_agent. This is a defense-in-depth gap, not a critical vulnerability. Otherwise the change is straightforward and the new test correctly validates the public-API guard.

✓ Test Coverage

The diff narrows type constraints from SupportsAgentRun to Agent across the handoff module and adds one test validating that participants() rejects non-Agent SupportsAgentRun instances. The test is well-structured with a meaningful assertion (TypeError match). However, coverage gaps exist: the runtime guard removed from _prepare_agent_with_handoffs has no replacement test, and other public API methods that changed signatures (add_handoff, with_start_agent) lack rejection tests. The constructor's participants= kwarg path is implicitly covered since it delegates to participants(), but that's not explicitly verified.

Suggestions

• In with_autonomous_mode, the docstring on line 947 was changed to say SupportsAgentRun / Agent instances but the type annotation was narrowed to Sequence[Agent]. This should just say Agent instances to be consistent with the rest of the refactor.
• The runtime isinstance(agent, Agent) check was removed from _prepare_agent_with_handoffs (the old lines 253-254). While the builder's participants() validates inputs, HandoffAgentExecutor can still be constructed directly without the builder, so callers bypassing the builder lose the runtime safety net. Consider keeping a runtime guard in __init__ or _prepare_agent_with_handoffs for defense-in-depth.
• Consider keeping a lightweight runtime isinstance check (or assert) at the top of HandoffAgentExecutor.init or _prepare_agent_with_handoffs. The old TypeError guard was removed (around old line 253), but HandoffAgentExecutor is a public class — callers that construct it directly bypass HandoffBuilder.participants() validation, and Python type hints are not enforced at runtime.
• Add a test that passes a non-Agent SupportsAgentRun to add_handoff(source=...) and add_handoff(targets=[...]) to verify those entry points also reject non-Agent instances consistently.
• Add a test that passes a non-Agent to with_start_agent() to verify the narrowed type constraint at that entry point.
• Consider adding a test for HandoffBuilder(participants=[fake]) (constructor kwarg path) to explicitly verify it delegates to the validated participants() method, even though it's implicitly covered.
• The runtime TypeError guard was removed from _prepare_agent_with_handoffs (old lines 253-254). If a non-Agent somehow bypasses the participants() check (e.g., via internal misuse), the error will now surface as an opaque failure in _clone_chat_agent. Consider adding a regression test or restoring a guard with a clear error message in _prepare_agent_with_handoffs.

Automated review by moonbox3's agents

Pull request overview

This PR fixes a type mismatch in HandoffBuilder where the API accepted SupportsAgentRun in type signatures but enforced Agent-only at runtime, causing late failures with confusing error messages. The fix narrows all public type annotations from SupportsAgentRun to Agent, moves validation into the participants() method with a clear error message, removes the redundant runtime check in _prepare_agent_with_handoffs, and adds a test confirming early rejection of non-Agent SupportsAgentRun implementors.

Changes:

• Updated type signatures to require Agent instead of SupportsAgentRun throughout HandoffBuilder and HandoffAgentExecutor
• Moved isinstance(participant, Agent) validation from _prepare_agent_with_handoffs to participants() with improved error messaging
• Added test to verify non-Agent SupportsAgentRun implementations are rejected early with a descriptive error

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.